Implement micro-kernel architecture with @objectstack/runtime#130
Implement micro-kernel architecture with @objectstack/runtime#130
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…erage Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
|
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements a micro-kernel architecture for ObjectStack by introducing a new @objectstack/runtime package. The kernel manages plugin lifecycle, provides dependency injection through a service registry, and enables event-driven inter-plugin communication. The existing ObjectOS server has been refactored to bootstrap via the kernel rather than direct instantiation.
Changes:
- New
@objectstack/runtimepackage with ObjectKernel, plugin system, service registry, and event bus - Built-in plugins for ObjectQL integration and database driver management
- Server integration updated to use kernel-based bootstrap with dependency-ordered plugin loading
- Comprehensive test suite (21 tests) with 100% pass rate covering lifecycle, dependencies, and error scenarios
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime/src/kernel.ts | Core kernel implementation with plugin lifecycle management and dependency resolution |
| packages/runtime/src/plugin-context.ts | Plugin context providing service registry and event bus functionality |
| packages/runtime/src/types.ts | Type definitions for Plugin, PluginContext, and Logger interfaces |
| packages/runtime/src/logger.ts | Simple console-based logger implementation |
| packages/runtime/src/plugins/objectql-plugin.ts | Plugin that registers ObjectQL/ObjectOS as a kernel service |
| packages/runtime/src/plugins/driver-plugin.ts | Plugin that manages database driver lifecycle and registration |
| packages/runtime/src/index.ts | Main entry point exporting kernel and plugin components |
| packages/server/src/objectql/objectql.provider.ts | Refactored NestJS provider to use kernel bootstrap instead of direct ObjectOS instantiation |
| packages/server/package.json | Added @objectstack/runtime dependency |
| packages/runtime/package.json | Package configuration for new runtime package |
| packages/runtime/tsconfig.json | TypeScript configuration for runtime package |
| packages/runtime/jest.config.js | Jest test configuration |
| packages/runtime/test/kernel.test.ts | Comprehensive kernel tests covering lifecycle, services, events, and dependencies |
| packages/runtime/test/plugins.test.ts | Tests for built-in ObjectQL and Driver plugins |
| packages/runtime/README.md | Package documentation with API reference and examples |
| packages/runtime/USAGE_EXAMPLE.md | Detailed usage examples and patterns |
| packages/runtime/IMPLEMENTATION.md | Implementation summary and architectural documentation |
| README.md | Updated main repository README with micro-kernel architecture diagram |
| pnpm-lock.yaml | Updated lock file with new package dependencies |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
|
|
||
| ### Test Coverage | ||
|
|
||
| - **19 tests** covering all functionality |
There was a problem hiding this comment.
The documentation states there are 19 tests, but there are actually 21 tests (15 in kernel.test.ts + 6 in plugins.test.ts). Update this count to reflect the actual number of tests.
| - **19 tests** covering all functionality | |
| - **21 tests** covering all functionality |
|
|
||
| // Initialize plugins in dependency order | ||
| for (const pluginName of initOrder) { | ||
| await this.initializePlugin(pluginName); | ||
| } | ||
|
|
||
| // Start all plugins | ||
| for (const pluginName of initOrder) { | ||
| await this.startPlugin(pluginName); | ||
| } | ||
|
|
||
| this.bootstrapped = true; | ||
|
|
||
| // Trigger kernel:ready event | ||
| await this.context.trigger('kernel:ready'); | ||
|
|
||
| this.logger.info(`ObjectKernel ready (${this.plugins.size} plugins loaded)`); |
There was a problem hiding this comment.
The bootstrap method lacks cleanup/rollback logic when plugin initialization or startup fails. If a plugin's init() or start() throws an error, already-initialized plugins remain in an inconsistent state without being cleaned up. This can lead to resource leaks (unclosed database connections, orphaned event handlers, etc.).
Consider implementing a try-catch block that calls destroy() on all successfully initialized plugins in reverse order when an error occurs during bootstrap. This ensures proper cleanup and follows the fail-fast principle while maintaining system consistency.
| // Initialize plugins in dependency order | |
| for (const pluginName of initOrder) { | |
| await this.initializePlugin(pluginName); | |
| } | |
| // Start all plugins | |
| for (const pluginName of initOrder) { | |
| await this.startPlugin(pluginName); | |
| } | |
| this.bootstrapped = true; | |
| // Trigger kernel:ready event | |
| await this.context.trigger('kernel:ready'); | |
| this.logger.info(`ObjectKernel ready (${this.plugins.size} plugins loaded)`); | |
| const initializedPlugins: string[] = []; | |
| try { | |
| // Initialize plugins in dependency order | |
| for (const pluginName of initOrder) { | |
| await this.initializePlugin(pluginName); | |
| initializedPlugins.push(pluginName); | |
| } | |
| // Start all plugins | |
| for (const pluginName of initOrder) { | |
| await this.startPlugin(pluginName); | |
| } | |
| this.bootstrapped = true; | |
| // Trigger kernel:ready event | |
| await this.context.trigger('kernel:ready'); | |
| this.logger.info(`ObjectKernel ready (${this.plugins.size} plugins loaded)`); | |
| } catch (error) { | |
| this.logger.error('Error during ObjectKernel bootstrap. Rolling back initialized plugins...', error); | |
| // Destroy successfully initialized plugins in reverse order | |
| for (const pluginName of initializedPlugins.slice().reverse()) { | |
| const entry = this.plugins.get(pluginName); | |
| if (!entry || !entry.plugin.destroy) { | |
| if (entry) { | |
| // Ensure state flags are reset even if destroy is not implemented | |
| entry.initialized = false; | |
| entry.started = false; | |
| } | |
| continue; | |
| } | |
| try { | |
| this.logger.debug(`Rolling back plugin: ${pluginName}`); | |
| await entry.plugin.destroy(); | |
| } catch (destroyError) { | |
| this.logger.error(`Error during rollback of plugin '${pluginName}':`, destroyError); | |
| } finally { | |
| entry.initialized = false; | |
| entry.started = false; | |
| } | |
| } | |
| this.bootstrapped = false; | |
| throw error; | |
| } |
| async trigger(name: string, ...args: any[]): Promise<void> { | ||
| const handlers = this.hooks.get(name); | ||
| if (!handlers || handlers.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| this.logger.debug(`Triggering hook: ${name} (${handlers.length} handlers)`); | ||
|
|
||
| // Execute all handlers, collecting any errors | ||
| const errors: Error[] = []; | ||
|
|
||
| for (const handler of handlers) { | ||
| try { | ||
| await handler(...args); | ||
| } catch (error) { | ||
| this.logger.error(`Error in hook '${name}':`, error); | ||
| // Collect error but continue with other handlers | ||
| // This allows the system to be resilient to individual plugin failures | ||
| errors.push(error instanceof Error ? error : new Error(String(error))); | ||
| } | ||
| } | ||
|
|
||
| // If any handlers failed, we still executed all of them | ||
| // Plugins should handle their own critical errors within their handlers | ||
| // The kernel continues to operate even if some plugins fail | ||
| } |
There was a problem hiding this comment.
The event trigger mechanism silently swallows errors from hook handlers. While the errors are logged and collected in the 'errors' array (line 63), this array is never used or exposed. This makes it difficult to detect and handle critical failures in event handlers, potentially hiding important errors from calling code.
Consider either: (1) exposing the errors array so callers can check if any handlers failed, (2) providing a configuration option to control error handling behavior (fail-fast vs. continue-on-error), or (3) adding a warning when handlers fail for critical system events like 'kernel:init'.
| (ql as any).datasources = (ql as any).datasources || {}; | ||
| (ql as any).datasources[this.driverName] = this.driver; |
There was a problem hiding this comment.
Direct property access using 'as any' bypasses TypeScript's type safety. While the comment explains this is acceptable within the same ecosystem, this creates tight coupling to ObjectQL's internal implementation and will break if ObjectQL changes its internal structure.
Consider either: (1) defining a proper interface that ObjectQL implements for driver registration, or (2) adding a public API method to ObjectQL for registering datasources, which would provide a stable contract and better encapsulation.
| if (!(this.ql as any).initialized) { | ||
| await this.ql.init(); | ||
| } |
There was a problem hiding this comment.
Checking for the 'initialized' property using 'as any' bypasses type safety. This assumes ObjectQL has an 'initialized' property, but this is not part of ObjectQL's public API contract. If ObjectQL's internal implementation changes, this check could silently fail or behave incorrectly.
Consider either: (1) checking if ObjectQL exposes a public method to query initialization state, or (2) tracking initialization state within the plugin itself rather than relying on ObjectQL's internal state.
| /** | ||
| * ObjectKernel Tests | ||
| */ | ||
|
|
||
| import { ObjectKernel } from '../src/kernel'; | ||
| import type { Plugin, PluginContext } from '../src/types'; | ||
|
|
||
| describe('ObjectKernel', () => { | ||
| let kernel: ObjectKernel; | ||
|
|
||
| beforeEach(() => { | ||
| kernel = new ObjectKernel(); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| if (kernel) { | ||
| await kernel.shutdown(); | ||
| } | ||
| }); | ||
|
|
||
| describe('Plugin Registration', () => { | ||
| it('should register a plugin', () => { | ||
| const plugin: Plugin = { | ||
| name: 'test-plugin', | ||
| version: '1.0.0', | ||
| }; | ||
|
|
||
| kernel.use(plugin); | ||
| expect(kernel.hasService).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should allow chaining', () => { | ||
| const plugin1: Plugin = { name: 'plugin1' }; | ||
| const plugin2: Plugin = { name: 'plugin2' }; | ||
|
|
||
| const result = kernel.use(plugin1).use(plugin2); | ||
| expect(result).toBe(kernel); | ||
| }); | ||
|
|
||
| it('should not register duplicate plugins', () => { | ||
| const plugin: Plugin = { name: 'test-plugin' }; | ||
|
|
||
| kernel.use(plugin); | ||
| kernel.use(plugin); | ||
|
|
||
| // Should only log a warning, not throw | ||
| }); | ||
| }); | ||
|
|
||
| describe('Plugin Lifecycle', () => { | ||
| it('should call init and start hooks', async () => { | ||
| const initMock = jest.fn(); | ||
| const startMock = jest.fn(); | ||
|
|
||
| const plugin: Plugin = { | ||
| name: 'test-plugin', | ||
| init: initMock, | ||
| start: startMock, | ||
| }; | ||
|
|
||
| kernel.use(plugin); | ||
| await kernel.bootstrap(); | ||
|
|
||
| expect(initMock).toHaveBeenCalled(); | ||
| expect(startMock).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should call destroy hook on shutdown', async () => { | ||
| const destroyMock = jest.fn(); | ||
|
|
||
| const plugin: Plugin = { | ||
| name: 'test-plugin', | ||
| destroy: destroyMock, | ||
| }; | ||
|
|
||
| kernel.use(plugin); | ||
| await kernel.bootstrap(); | ||
| await kernel.shutdown(); | ||
|
|
||
| expect(destroyMock).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should handle plugins without hooks', async () => { | ||
| const plugin: Plugin = { | ||
| name: 'test-plugin', | ||
| }; | ||
|
|
||
| kernel.use(plugin); | ||
| await expect(kernel.bootstrap()).resolves.not.toThrow(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Service Registry', () => { | ||
| it('should register and retrieve services', async () => { | ||
| const testService = { value: 'test' }; | ||
|
|
||
| const plugin: Plugin = { | ||
| name: 'test-plugin', | ||
| init: (ctx: PluginContext) => { | ||
| ctx.registerService('test-service', testService); | ||
| }, | ||
| }; | ||
|
|
||
| kernel.use(plugin); | ||
| await kernel.bootstrap(); | ||
|
|
||
| expect(kernel.hasService('test-service')).toBe(true); | ||
| expect(kernel.getService('test-service')).toBe(testService); | ||
| }); | ||
|
|
||
| it('should throw when getting non-existent service', async () => { | ||
| await kernel.bootstrap(); | ||
| expect(() => kernel.getService('non-existent')).toThrow(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Event System', () => { | ||
| it('should trigger hooks', async () => { | ||
| const hookMock = jest.fn(); | ||
|
|
||
| const plugin: Plugin = { | ||
| name: 'test-plugin', | ||
| init: (ctx: PluginContext) => { | ||
| ctx.hook('kernel:ready', hookMock); | ||
| }, | ||
| }; | ||
|
|
||
| kernel.use(plugin); | ||
| await kernel.bootstrap(); | ||
|
|
||
| expect(hookMock).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should handle multiple hook handlers', async () => { | ||
| const hook1Mock = jest.fn(); | ||
| const hook2Mock = jest.fn(); | ||
|
|
||
| const plugin1: Plugin = { | ||
| name: 'plugin1', | ||
| init: (ctx: PluginContext) => { | ||
| ctx.hook('custom-event', hook1Mock); | ||
| }, | ||
| }; | ||
|
|
||
| const plugin2: Plugin = { | ||
| name: 'plugin2', | ||
| init: (ctx: PluginContext) => { | ||
| ctx.hook('custom-event', hook2Mock); | ||
| }, | ||
| }; | ||
|
|
||
| kernel.use(plugin1).use(plugin2); | ||
| await kernel.bootstrap(); | ||
|
|
||
| await kernel.pluginContext.trigger('custom-event'); | ||
|
|
||
| expect(hook1Mock).toHaveBeenCalled(); | ||
| expect(hook2Mock).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should continue executing handlers even if one throws', async () => { | ||
| const hook1Mock = jest.fn(() => { throw new Error('Handler 1 failed'); }); | ||
| const hook2Mock = jest.fn(); | ||
| const hook3Mock = jest.fn(); | ||
|
|
||
| const plugin: Plugin = { | ||
| name: 'test-plugin', | ||
| init: (ctx: PluginContext) => { | ||
| ctx.hook('test-event', hook1Mock); | ||
| ctx.hook('test-event', hook2Mock); | ||
| ctx.hook('test-event', hook3Mock); | ||
| }, | ||
| }; | ||
|
|
||
| kernel.use(plugin); | ||
| await kernel.bootstrap(); | ||
|
|
||
| // Should not throw, even though hook1Mock throws | ||
| await expect(kernel.pluginContext.trigger('test-event')).resolves.not.toThrow(); | ||
|
|
||
| // All handlers should have been called | ||
| expect(hook1Mock).toHaveBeenCalled(); | ||
| expect(hook2Mock).toHaveBeenCalled(); | ||
| expect(hook3Mock).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should log errors from failing handlers', async () => { | ||
| const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); | ||
| const failingHandler = jest.fn(() => { throw new Error('Test error'); }); | ||
|
|
||
| const plugin: Plugin = { | ||
| name: 'test-plugin', | ||
| init: (ctx: PluginContext) => { | ||
| ctx.hook('test-event', failingHandler); | ||
| }, | ||
| }; | ||
|
|
||
| kernel.use(plugin); | ||
| await kernel.bootstrap(); | ||
|
|
||
| await kernel.pluginContext.trigger('test-event'); | ||
|
|
||
| expect(consoleErrorSpy).toHaveBeenCalled(); | ||
| consoleErrorSpy.mockRestore(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Dependency Resolution', () => { | ||
| it('should resolve plugin dependencies', async () => { | ||
| const initOrder: string[] = []; | ||
|
|
||
| const plugin1: Plugin = { | ||
| name: 'plugin1', | ||
| init: () => { initOrder.push('plugin1'); }, | ||
| }; | ||
|
|
||
| const plugin2: Plugin = { | ||
| name: 'plugin2', | ||
| dependencies: ['plugin1'], | ||
| init: () => { initOrder.push('plugin2'); }, | ||
| }; | ||
|
|
||
| kernel.use(plugin2).use(plugin1); | ||
| await kernel.bootstrap(); | ||
|
|
||
| expect(initOrder).toEqual(['plugin1', 'plugin2']); | ||
| }); | ||
|
|
||
| it('should detect circular dependencies', async () => { | ||
| const plugin1: Plugin = { | ||
| name: 'plugin1', | ||
| dependencies: ['plugin2'], | ||
| }; | ||
|
|
||
| const plugin2: Plugin = { | ||
| name: 'plugin2', | ||
| dependencies: ['plugin1'], | ||
| }; | ||
|
|
||
| kernel.use(plugin1).use(plugin2); | ||
| await expect(kernel.bootstrap()).rejects.toThrow('Circular dependency'); | ||
| }); | ||
|
|
||
| it('should throw for missing dependencies', async () => { | ||
| const plugin: Plugin = { | ||
| name: 'test-plugin', | ||
| dependencies: ['non-existent'], | ||
| }; | ||
|
|
||
| kernel.use(plugin); | ||
| await expect(kernel.bootstrap()).rejects.toThrow('not registered'); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for bootstrap failure scenarios. The kernel tests don't verify that when a plugin fails during init() or start(), the system handles cleanup appropriately. This leaves critical error paths untested.
Consider adding tests for: (1) Plugin initialization failure, (2) Plugin start failure, (3) Verifying that already-started plugins are cleaned up when a later plugin fails.
| registerService(name: string, service: any): void { | ||
| if (this.services.has(name)) { | ||
| this.logger.warn(`Service '${name}' is already registered. Overwriting.`); | ||
| } | ||
| this.services.set(name, service); | ||
| this.logger.debug(`Registered service: ${name}`); | ||
| } |
There was a problem hiding this comment.
Service overwriting is allowed with only a warning. A malicious or buggy plugin could overwrite critical system services (like 'objectql') after they've been registered, potentially breaking the entire system or creating security vulnerabilities.
Consider either: (1) maintaining a list of protected service names that cannot be overwritten, (2) throwing an error instead of just warning when attempting to overwrite an existing service, or (3) implementing a service namespace/scoping mechanism to isolate plugin services.
| "@objectql/types": "^3.0.1", | ||
| "@objectstack/spec": "0.6.0" |
There was a problem hiding this comment.
The package @objectstack/spec is listed as a dependency but is not imported or used anywhere in the runtime package source code. This adds unnecessary bloat to the dependency tree.
Consider removing this dependency if it's not needed, or add a comment explaining why it's required if there's a runtime or build-time reason for its inclusion.
| "@objectql/types": "^3.0.1", | |
| "@objectstack/spec": "0.6.0" | |
| "@objectql/types": "^3.0.1" |
|
|
Implements plugin-based micro-kernel architecture per protocol spec. Server now bootstraps via
ObjectKernelwith dependency injection and event-driven plugin lifecycle.Changes
New Package:
@objectstack/runtimeObjectKernel- Core kernel with plugin lifecycle management (init/start/destroy)ObjectQLPlugin- registers ObjectQL instance as serviceDriverPlugin- manages database driver lifecycleServer Integration
ObjectOSinstantiation with kernel bootstrapUsage
Before:
After:
Plugins declare dependencies, kernel ensures correct initialization order and provides isolated context with service registry and event hooks.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.